-
Notifications
You must be signed in to change notification settings - Fork 131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix auto-reshim on bundle install and gem uninstall. #162
Conversation
d4f59aa
to
08a2f79
Compare
else | ||
maybe_reshim = lambda do |installer| | ||
# If any gems with executables were installed or uninstalled, reshim. | ||
`asdf reshim ruby` if installer.spec.executables.any? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not specify the RUBY_VERSION
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand why the bin/${executable}
part was removed, but the version might still be worth including.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is a bug with asdf itself, but while the #{RUBY_VERSION}
does seem to work after gem install
, it does not remove the dead shim after gem uninstall
.
With version:
$ gem install railties ; echo "Rails: $(which rails)" ; yes | gem uninstall --force railties; echo "Rails: $(which rails)"
Fetching railties-6.0.2.2.gem
Successfully installed railties-6.0.2.2
Parsing documentation for railties-6.0.2.2
Installing ri documentation for railties-6.0.2.2
Done installing documentation for railties after 0 seconds
1 gem installed
Rails: /Users/djmarcin/.asdf/shims/rails
Removing rails
Successfully uninstalled railties-6.0.2.2
Rails: /Users/djmarcin/.asdf/shims/rails
Without version:
$ gem install railties ; echo "Rails: $(which rails)" ; yes | gem uninstall --force railties; echo "Rails: $(which rails)"
Fetching railties-6.0.2.2.gem
Successfully installed railties-6.0.2.2
Parsing documentation for railties-6.0.2.2
Installing ri documentation for railties-6.0.2.2
Done installing documentation for railties after 0 seconds
1 gem installed
Rails: /Users/djmarcin/.asdf/shims/rails
Removing rails
Successfully uninstalled railties-6.0.2.2
Rails:
Although, since installs are likely more frequent than uninstalls, I could change the behavior to iterate over the binaries on install, and just reshim everything on uninstall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the customized behavior for install & uninstall.
|
||
if installer.spec.executables.any? && installer.bin_dir == Gem.default_bindir | ||
if defined?(Bundler::Installer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is defined, wouldn't we still want the Gem
hooks setup too? It doesn't seem like these two things are mutually exclusive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that Bundler::Installer
is only defined when the bundle
command is running. While Bundler is operating, we wouldn't want the gem hooks to run because we want to batch one reshim operation at the end rather than after each Gem.
This fixes #63 but depends on rubygems/rubygems#3534 being merged in order to actually work. There is a long-standing bug in bundler that prevents it from loading
rubygems_plugin.rb
fromRUBYLIB
. However, there is no problem having this hook exist with a version of bundler that doesn't support it -- this file will just never be evaluated in that case.It also fixes shims not being removed on
gem uninstall
where installer.bin_dir is not set, so it can never matchGem.default_bindir
. Additionally, the#{RUBY_VERSION} bin/#{executable}
syntax does not work for removing shims, so in the interest of simplicity, the hook now just runsasdf reshim ruby
which works in both the install and uninstall cases, but is potentially less efficient for installation viagem install
.